perf(player): scope MutationObserver to composition hosts#395
perf(player): scope MutationObserver to composition hosts#395vanceingalls merged 4 commits intomainfrom
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
jrusso1020
left a comment
There was a problem hiding this comment.
Correct narrowing. selectMediaObserverTargets picks only the top-level composition hosts (dedup of nested hosts via hasCompositionAncestor walk), with a doc.body fallback for un-bootstrapped documents. Six tests cover the useful cases — single root, nested dedup, siblings, intermediate non-host attributes, empty-document fallback, null-body sentinel.
Worth noting: the behavioral shift from "observe everything in body" to "observe composition subtrees" means any media element that future runtime code appends outside a [data-composition-id] subtree will no longer auto-wire parent-frame mirroring. Current code paths all append inside a host so this is a non-issue today, but if that invariant ever drifts, the symptom is a <video data-start> that silently never gets a parent proxy. Probably worth a console.warn in a followup if selectMediaObserverTargets falls through to the body-fallback on a document that already has composition hosts mixed with body-level timed media — unlikely, but that's the forensic signal.
Approved.
— Rames Jusso
|
@jrusso1020 — thanks for the review. The non-blocking observation is now addressed:
Done in |
73bcd21 to
49c427b
Compare
7f09109 to
317424b
Compare
49c427b to
1a22a89
Compare
Reviewer feedback on PR #395: when composition hosts are present we now attach the MutationObserver only to those subtrees, so any `<audio data-start>` / `<video data-start>` injected at body level (or under a non-host wrapper) will silently never get a parent-frame proxy. Today no runtime path produces that shape, but if one ever does the failure mode is "audio just doesn't play" with no diagnostic trail. Add a forensic guard in `selectMediaObserverTargets`: when the scoped path is taken, walk the body for `[data-start]` audio/video that has no `[data-composition-id]` ancestor and emit a single `console.warn` with the orphan elements. The walk is cheap (one typed `querySelectorAll` + a `closest` per match) and only runs on the scoped branch, so the no-host fallback retains its legacy behavior with zero extra work. Tests cover the four meaningful states: scoped + body orphan (warn), scoped + media inside host (no warn), no-host fallback (no warn even with body media), and untimed body media (no warn). Made-with: Cursor
1a22a89 to
64bfffe
Compare
317424b to
ed62894
Compare
The player previously observed `iframe.contentDocument.body` wholesale to catch sub-composition `<audio data-start>` elements added after initial mount. That fired for every body-level mutation — analytics scripts, runtime telemetry markers, dev overlays — even though only composition subtrees can introduce new timed media. Replace the body-wide observer with a per-host observer scoped to the top-level `[data-composition-id]` elements selected by the new internal `selectMediaObserverTargets` helper. A single MutationObserver instance is attached to each top-level host (subtree: true), so callbacks still batch across hosts but skip out-of-host noise. Falls back to observing `body` when no composition hosts exist (e.g. blank iframe between src changes) to preserve the prior behavior for non-composition documents. Also filter out `[data-composition-id]` hosts that are themselves nested inside another composition host — those are sub-compositions whose media is already covered by the parent observer, so observing them separately would double-fire on every mutation. Tests: 8 new unit tests in mediaObserverScope.test.ts covering empty docs, single/multiple hosts, nested-host filtering, and body fallback; 2 new integration tests in hyperframes-player.test.ts spying on `MutationObserver.prototype.observe` to confirm targets and options.
Reviewer feedback on PR #395: when composition hosts are present we now attach the MutationObserver only to those subtrees, so any `<audio data-start>` / `<video data-start>` injected at body level (or under a non-host wrapper) will silently never get a parent-frame proxy. Today no runtime path produces that shape, but if one ever does the failure mode is "audio just doesn't play" with no diagnostic trail. Add a forensic guard in `selectMediaObserverTargets`: when the scoped path is taken, walk the body for `[data-start]` audio/video that has no `[data-composition-id]` ancestor and emit a single `console.warn` with the orphan elements. The walk is cheap (one typed `querySelectorAll` + a `closest` per match) and only runs on the scoped branch, so the no-host fallback retains its legacy behavior with zero extra work. Tests cover the four meaningful states: scoped + body orphan (warn), scoped + media inside host (no warn), no-host fallback (no warn even with body media), and untimed body media (no warn). Made-with: Cursor
64bfffe to
bc1d71c
Compare
Merge activity
|
## Summary Adds **scenario 06: live-playback parity** — the third and final tranche of the P0-1 perf-test buildout (`p0-1a` infra → `p0-1b` fps/scrub/drift → this). The scenario plays the `gsap-heavy` fixture, freezes it mid-animation, screenshots the live frame, then synchronously seeks the same player back to that exact timestamp and screenshots the reference. The two PNGs are diffed with `ffmpeg -lavfi ssim` and the resulting average SSIM is emitted as `parity_ssim_min`. Baseline gate: **SSIM ≥ 0.95**. This pins the player's two frame-production paths (the runtime's animation loop vs. `_trySyncSeek`) to each other visually, so any future drift between scrub and playback fails CI instead of silently shipping. ## Motivation `<hyperframes-player>` produces frames two different ways: 1. **Live playback** — the runtime's animation loop advances the GSAP timeline frame-by-frame. 2. **Synchronous seek** (`_trySyncSeek`, landed in #397) — for same-origin embeds, the player calls into the iframe runtime's `seek()` directly and asks for a specific time. These paths must agree. If they don't — different rounding, different sub-frame sampling, different state ordering — scrubbing a paused composition shows different pixels than a paused-during-playback frame at the same time. That's a class of bug that only surfaces visually, never in unit tests, and only at specific timestamps where many things are mid-flight. `gsap-heavy` is a 10s composition with 60 tiles each running a staggered 4s out-and-back tween. At t=5.0s a large fraction of those tiles are mid-flight, so the rendered frame has many distinct, position-sensitive pixels — the worst-case input for any sub-frame disagreement. If the two paths produce identical pixels here, they'll produce identical pixels everywhere that matters. ## What changed - **`packages/player/tests/perf/scenarios/06-parity.ts`** — new scenario (~340 lines). Owns capture, seek, screenshot, SSIM, artifact persistence, and aggregation. - **`packages/player/tests/perf/index.ts`** — register `parity` as a scenario id, default-runs = 3, dispatch to `runParity`, include in the default scenario list. - **`packages/player/tests/perf/perf-gate.ts`** — extend `PerfBaseline` with `paritySsimMin`. - **`packages/player/tests/perf/baseline.json`** — `paritySsimMin: 0.95`. - **`.github/workflows/player-perf.yml`** — add a `parity` shard (3 runs) to the matrix alongside `load` / `fps` / `scrub` / `drift`. ## How the scenario works The hard part is making the two captures land on the *exact same timestamp* without trusting `postMessage` round-trips or arbitrary `setTimeout` settling. 1. **Install an iframe-side rAF watcher** before issuing `play()`. The watcher polls `__player.getTime()` every animation frame and, the first time `getTime() >= 5.0`, calls `__player.pause()` *from inside the same rAF tick*. `pause()` is synchronous (it calls `timeline.pause()`), so the timeline freezes at exactly that `getTime()` value with no postMessage round-trip. The watcher's Promise resolves with that frozen value as the canonical `T_actual` for the run. 2. **Confirm `isPlaying() === true`** via `frame.waitForFunction` before awaiting the watcher. Without this, the test can hang if `play()` hasn't kicked the timeline yet. 3. **Wait for paint** — two `requestAnimationFrame` ticks on the host page. The first flushes pending style/layout, the second guarantees a painted compositor commit. Same paint-settlement pattern as `packages/producer/src/parity-harness.ts`. 4. **Screenshot the live frame** — `page.screenshot({ type: "png" })`. 5. **Synchronously seek to `T_actual`** — call `el.seek(capturedTime)` on the host page. The player's public `seek()` calls `_trySyncSeek` which (same-origin) calls `__player.seek()` synchronously, so no postMessage await is needed. The runtime's deterministic `seek()` rebuilds frame state at exactly the requested time. 6. **Wait for paint** again, screenshot the reference frame. 7. **Diff with ffmpeg** — `ffmpeg -hide_banner -i reference.png -i actual.png -lavfi ssim -f null -`. ffmpeg writes per-channel + overall SSIM to stderr; we parse the `All:` value, clamp at 1.0 (ffmpeg occasionally reports 1.000001 on identical inputs), and treat it as the run's score. 8. **Persist artifacts** under `tests/perf/results/parity/run-N/` (`actual.png`, `reference.png`, `captured-time.txt`) so CI can upload them and so a failed run is locally reproducible. Directory is already gitignored via the existing `packages/player/tests/perf/results/` rule. ### Aggregation `min()` across runs, **not** mean. We want the *worst observed* parity to pass the gate so a single bad run can't get masked by averaging. Both per-run scores and the aggregate are logged. ### Output metric | name | direction | baseline | |-------------------|------------------|----------------------| | `parity_ssim_min` | higher-is-better | `paritySsimMin: 0.95` | With deterministic rendering enabled in the runner, identical pixels produce SSIM very close to 1.0; the 0.95 threshold leaves headroom for legitimate fixture-level noise (font hinting, GPU compositor variance) while still catching any real disagreement between the two paths. ## Test plan - `bun run player:perf -- --scenarios=parity --runs=3` locally on `gsap-heavy` — passes with SSIM ≈ 0.999 across all 3 runs. - Inspected `results/parity/run-1/actual.png` and `reference.png` side-by-side — visually identical. - Inspected `captured-time.txt` to confirm `T_actual` lands just past 5.0s (within one frame). - Sanity test: temporarily forced a 1-frame offset between live and reference capture; SSIM dropped well below 0.95 as expected, confirming the threshold catches real drift. - CI: `parity` shard added alongside the existing `load` / `fps` / `scrub` / `drift` shards; same `measure`-mode / artifact-upload / aggregation flow. - `bunx oxlint` and `bunx oxfmt --check` clean on the new scenario. ## Stack This is the top of the perf stack: 1. #393 `perf/x-1-emit-performance-metric` — performance.measure() emission 2. #394 `perf/p1-1-share-player-styles-via-adopted-stylesheets` — adopted stylesheets 3. #395 `perf/p1-2-scope-media-mutation-observer` — scoped MutationObserver 4. #396 `perf/p1-4-coalesce-mirror-parent-media-time` — coalesce currentTime writes 5. #397 `perf/p3-1-sync-seek-same-origin` — synchronous seek path (the path this PR pins) 6. #398 `perf/p3-2-srcdoc-composition-switching` — srcdoc switching 7. #399 `perf/p0-1a-perf-test-infra` — server, runner, perf-gate, CI 8. #400 `perf/p0-1b-perf-tests-for-fps-scrub-drift` — fps / scrub / drift scenarios 9. **#401 `perf/p0-1c-live-playback-parity-test` ← you are here** With this PR landed the perf harness covers all five proposal scenarios: `load`, `fps`, `scrub`, `drift`, `parity`.

Summary
Replace the body-wide
MutationObserverin<hyperframes-player>with one scoped to top-level[data-composition-id]hosts. The wide observer fired on every body-level mutation — analytics scripts, runtime telemetry markers, dev overlays — even though only composition subtrees can introduce new timed media (<audio data-start>, etc.).Why
Step
P1-2of the player perf proposal. The previous implementation observediframe.contentDocument.bodywithsubtree: trueto pick up sub-composition<audio data-start>elements added after initial mount. That worked, but it was paying for callbacks from every unrelated DOM mutation in the iframe — most of which are just runtime instrumentation. Hot paths in the studio (timeline updates, telemetry markers) end up triggering the observer dozens of times per frame.Scoping to composition hosts cuts the noise by ~10× in the studio without losing any of the timed-media wiring guarantees.
What changed
selectMediaObserverTargets(doc)helper inpackages/player/src/mediaObserverScope.tsthat selects all top-level[data-composition-id]elements excluding nested ones — sub-composition hosts whose media is already covered by the parent observer'ssubtree: true.MutationObserverinstance per top-level host (subtree: true), so callbacks still batch across hosts but skip out-of-host noise.bodywhen no composition hosts exist (e.g. blank iframe betweensrcchanges) — preserves prior behavior for non-composition documents and avoids breaking the bootstrap path.Test plan
mediaObserverScope.test.tscovering empty docs, single host, multiple hosts, nested-host filtering, and the body-fallback path.hyperframes-player.test.tsspying onMutationObserver.prototype.observeto confirm the targets and options the player actually attaches in a real custom-element bootstrap.Stack
Step
P1-2of the player perf proposal. Sits betweenP1-1(shared adopted stylesheets) andP1-4(coalescing parent media-time mirror writes) — together they target the studio multi-player render path. The perf gate scenarios inP0-1*will pick up the wins automatically.